All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Code review

Solid model design (CellSelection with O(1) column-select, no materialization of 10k+ positions) and the row/cell selection mutex is wired in the right places. Below are the issues to address before merge.

Blockers

1. copyRowsAsTSV validation is wrong. validateUserInterfaceItem now enables copyRowsAsTSV(_:) whenever cellSelection is non-empty, but copyRowsAsTSV only handles selectedRowIndexes. With cell-only selection, the menu item is enabled but copies nothing.

case #selector(copy(_:)), #selector(copyRowsAsTSV(_:)):
    return !selection.cellSelection.isEmpty || !selectedRowIndexes.isEmpty

Either route copyRowsAsTSV through the cell-selection path too, or revert that case to !selectedRowIndexes.isEmpty.

2. Clipboard is plain text only, not TSV. Newline-joined plain text means a multi-cell paste into Excel/Numbers/SQL editors lands in one cell. The codebase already has a TSV writer in copyRowsAsTSV. Write both public.utf8-plain-text and public.tab-separated-values representations to NSPasteboard so it pastes correctly everywhere. This also makes cell-copy consistent with row-copy.

3. Cmd+Shift-click on a header cancels multi-sort.

// SortableHeaderView.swift:188
if modifiers.contains(.command) {
    coordinator.selectColumn(dataIndex)
    return
}

This matches .command even when .shift is held. The table-view path is explicit about this: modifiers.contains(.command) && !modifiers.contains(.shift). Mirror that here, otherwise Cmd+Shift+header-click (a plausible multi-sort attempt) is hijacked into column-select.

4. No tests. CellSelection is pure-Swift and trivially testable (contains, affectedRows, affectedColumns, isEmpty), and TableSelection.reloadIndexes has a new cell-selection branch worth covering. CLAUDE.md mandates tests for testable features; the PR description noting "Local build skipped (no Xcode locally), relying on CI" doesn't substitute. CI runs tests, it doesn't write them.

5. No docs update. New shortcuts (Cmd-click header, Shift-click cell, Cmd-click cell, Esc) need entries in docs/features/keyboard-shortcuts.mdx and the data-grid feature page. Mandatory per CLAUDE.md.

UX call worth confirming

Shift-click after a column selection collapses to a single cell. handleShiftClickCell requires cellSelectionAnchor != nil, but selectColumn sets the anchor to nil. So the flow Cmd+click header → Shift+click a cell in that column drops to a single-cell selection instead of extending into a row range. Either treat .column as an implicit anchor (e.g., row 0), or document this as not supported.

Lower-priority observations

• CellSelection.swift is a model under Views/Results/. Consider Models/UI/CellSelection.swift to match the pattern used by PhpTreeNode.swift.
• Silent skip on displayRow(at:) failure in copyCellRange / copyCellPositions — partial copy with no signal can produce wrong SQL in the "paste into WHERE clause" use case. At minimum add an OSLog warn.
• CHANGELOG entry reads a bit long. Suggest: Column and cell-range selection in the data grid via Cmd-click header, Shift-click range, Cmd-click cells (#1446).
• PR diverges from the issue's "click column header" wording (uses Cmd+click so plain click still sorts). Defensible — matches DataGrip/Postico/Excel — but worth calling out in the PR body so the divergence isn't lost.
• The visual 12% accent tint stacks with the existing modified-column tint. Worth a manual look in the running app.
• KeyHandlingTableView.swift grew ~86 lines. Check against SwiftLint's 1200-line warning; if it's close, extract the mouseDown routing into KeyHandlingTableView+CellSelection.swift.

Build / verification

PR test plan is unchecked. Code looks compile-safe, but the mouse-routing and Escape changes need a manual pass through the listed scenarios — especially the 10k+ row column-select non-freeze and the inline-edit-still-works case — before merging.

Thanks for the thorough review! All blockers and the UX suggestion are addressed in the latest push:

1. copyRowsAsTSV validation - split into its own case, only checks selectedRowIndexes
2. Clipboard TSV format - copyCellRange and copyCellPositions now use writeRows(tsv:html:) so paste into Excel/Numbers lands correctly
3. Cmd+Shift header click - added && !modifiers.contains(.shift) so Cmd+Shift still triggers multi-sort
4. Tests - added CellSelectionTests.swift covering contains, affectedRows, affectedColumns, isEmpty for all cases, plus TableSelection.reloadIndexes cell-selection branch
5. Docs - updated docs/features/keyboard-shortcuts.mdx with the new selection shortcuts
6. UX: Shift+click after column select - selectColumn now sets an implicit anchor at row 0 so Shift+click can extend into a range

Code Review

Overview

Adds three new selection modes to the data grid (full column, vertical range, non-contiguous cells), wired through a new CellSelection enum stored on TableSelection. Selection state drives a 12% accent tint, Cmd+C copy, and Escape clear.

🔴 Critical: Won't compile

DataGridView+RowActions.swift calls ClipboardService.writeRows with the wrong signature.

The protocol (TablePro/Core/Services/Infrastructure/ClipboardService.swift:20) is:

func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload)

But this PR calls:

ClipboardService.shared.writeRows(tsv: lines.joined(separator: "\n"), html: nil)

Missing gridRows:. This will fail xcodebuild. Fix by either:

• Switching to writeText(...) (matches copyColumnValues and is the right choice — a single column of values isn't a tabular grid), or
• Providing a real GridRowsClipboardPayload so external paste round-trips work like row copy.

The writeText route is the smaller, more consistent change.

🟡 Correctness / behavior concerns

• Stale cellSelectionAnchor after reload. The anchor lives on KeyHandlingTableView as a bare var, not in TableSelection. When the underlying dataset changes (new query, filter, sort), selection is reset but cellSelectionAnchor is not, so a stale (row, column) can drive the next shift+click. Either move the anchor into TableSelection so resets clear it together, or clear it whenever the rows change.

• cancelOperation should still call super. It now only handles the cell-selection-clear case and silently returns otherwise. Forwarding to super.cancelOperation when there's nothing to clear is more correct (and matches the AppKit chain).

• Header has no visual indication a column is selected. selectColumn tints cells but SortableHeaderView doesn't reflect the state. Users who scroll horizontally or focus elsewhere lose the cue. Worth painting a subtle tint behind the column header too.

• Shift+click in a different column silently switches anchor. handleShiftClickCell replaces the selection with a single cell when the anchor column doesn't match. That matches the docs but a user who shift+clicked the wrong column loses their prior anchor with no feedback. Consider treating cross-column shift+click as a no-op so the original anchor survives.

• affectedRows empty for .column is load-bearing. TableSelection.reloadIndexes returns nil for column-only transitions, and the actual reload is driven by the side-channel reloadVisibleColumnCells in selection.didSet. It works, but the two-path design is fragile — columnChangeReturnsNilDueToEmptyRows enshrines the quirk in a test rather than fixing it. Consider making affectedRows return all rows for .column and removing the side-channel.

🟢 Style / convention nits

• Hardcoded 0.12 accent alpha at DataGridCellView.swift:211. The file already uses modifiedColumnTint resolved from the theme palette. Add a cellSelectionTint to the palette so light/dark and theme switching stay consistent.

• handleCmdClickCell switch is redundant. Lines 167–172 collapse .column, .range, and .none to the same empty Set(). Simplify:

  var positions: Set<CellPosition> = {
      if case .cells(let existing) = selection.cellSelection { return existing }
      return []
  }()

• Duplicated copy plumbing. copyCellRange, copyCellPositions, and the existing copyColumnValues share the same "look up column type → format cell → join with \n" loop. After fixing the API call, extract a small helper that takes [(row: Int, column: Int)] and returns the joined string.

• Tests cover the model, not the interactive layer. CellSelectionTests is solid for the enum. There are no tests for copyCellSelection formatting (especially multi-row/multi-column .cells, which currently flattens rather than producing a 2D grid). Worth adding before this ships.

Other notes

• CHANGELOG, docs (keyboard-shortcuts.mdx), and conventional commit scope (datagrid) all look right.
• No localization concerns.
• Cmd+click on column header doesn't conflict with existing header shortcuts (Shift+click is multi-sort, plain click cycles sort).

Recommendation

Request changes. The writeRows call is a build-breaker — fix that first. Anchor lifetime and the theme-vs-hardcoded tint are worth addressing in the same pass.